Improvements for dshackle chart
#1
Conversation
cristidas
requested review from
eskp,
OleksandrUA,
nickkounz,
sanbotto and
jeannettemcd
| @@ -60,3 +60,10 @@ Create the name of the service account to use | |||
| {{- default "default" .Values.serviceAccount.name }} | |||
| {{- end }} | |||
| {{- end }} | |||
|
|
|||
| {{/* | |||
| HTTP port | |||
There was a problem hiding this comment.
We could also change to HTTPS, but throughout this chart http is used to label that specific traffic
There was a problem hiding this comment.
http here represents internal port which is not HTTPS, internally they communicate over HTTP on port 8545
| - port: {{ .Values.httpPort }} | ||
| targetPort: http | ||
| - port: {{ .Values.externalHttpPort }} | ||
| targetPort: {{ .Values.internalHttpPort }} |
There was a problem hiding this comment.
@cristidas I think if we plan to open PR to back-merge it we should make as less changes as possible. I would suggest to introduce externalHttpPort but keep using httpPort as internal one
There was a problem hiding this comment.
@OleksandrUA it wouldn't be all these changes in a PR, and I've already opened one only for the changes related to service.type: LoadBalancer. We'll see after that if we'll need to refactor the changes for the ports and create a PR to upstream
dshackle.httpPortannotationssupport for configuring a Service of typeLoadBalancer